Skip to content

Conversation

BSynRedhat
Copy link
Contributor

@BSynRedhat BSynRedhat commented Aug 8, 2025

Update to insert a section between current 6.1 and 6.2 about allowing offline access to online and external code sources.

Summary by CodeRabbit

  • Documentation
    • Added guidance for enabling external resource access for LMEval jobs, including security warnings and both CLI and web console procedures.
    • Introduced per-job and cluster-level configuration steps for online access and remote code execution.
    • Updated evaluation docs to include the new section.
    • Renamed global setting to lmes-allow-code-execution and set online/code execution to disabled by default.
    • Simplified setup guidance and removed outdated enablement steps.

Copy link

coderabbitai bot commented Aug 8, 2025

Walkthrough

Adds a new assembly documenting external resource access for LMEval jobs, includes two new procedure modules (CLI and web console), updates the main evaluation assembly to include the new section, and revises LM-Eval global settings in setup docs (renamed keys, defaults changed, procedural steps removed).

Changes

Cohort / File(s) Summary of changes
New assembly: external access
assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc
New ASSEMBLY describing enabling online access and remote code execution for LMEval jobs; includes CLI and web console modules.
Assembly include update
assemblies/evaluating-large-language-models.adoc
Added include for the new external access assembly between existing sections.
LM-Eval settings revision
modules/setting-up-lmeval.adoc
Renamed setting key to lmes-allow-code-execution; set defaults lmes-allow-online=false and lmes-allow-code-execution=false; minor formatting; removed prior procedural enablement guidance, retaining concise policy and restart note.
New procedure: CLI
modules/updating-lmeval-job-configuration-using-the-cli.adoc
Added procedure to enable allowOnline and allowCodeExecution via CLI: patch operator ConfigMap, restart operator, and set per-job fields; includes commands and YAML examples with security cautions.
New procedure: Web console
modules/updating-lmeval-job-configuration-using-the-web-console.adoc
Added procedure to enable allowOnline and allowCodeExecution via web console: edit ConfigMap, restart rollout, set per-job fields; includes sample YAML and a config keys table with defaults and notes.

Sequence Diagram(s)

sequenceDiagram
  participant Admin
  participant OpenShift/Cluster
  participant TrustyAI Operator
  participant LMEvalJob

  Admin->>OpenShift/Cluster: Patch trustyai-service-operator-config (lmes-allow-online/code-execution=true)
  Admin->>OpenShift/Cluster: Restart operator deployment
  OpenShift/Cluster->>TrustyAI Operator: Apply new config
  Admin->>OpenShift/Cluster: Create/Patch LMEvalJob (allowOnline/allowCodeExecution=true)
  TrustyAI Operator->>LMEvalJob: Reconcile with updated settings
  LMEvalJob->>External Resources: Access datasets/tokenizers (if enabled)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (2)
modules/setting-up-lmeval.adoc (1)

75-83: Ensure ConfigMap key is lmes-allow-code-execution, not lmes-code-execution

The console and CLI procedures use lmes-allow-code-execution, so we should update all docs for consistency:

• modules/setting-up-lmeval.adoc (around line 79): change lmes-code-executionlmes-allow-code-execution
• modules/using-llm-as-a-judge-metrics-with-lmeval.adoc (around line 68): change lmes-code-executionlmes-allow-code-execution

Replace the table rows in each file as follows:

-| `lmes-code-execution`
-| true
-| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+| `lmes-allow-code-execution`
+| true
+| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
modules/updating-lmeval-job-configuration-using-the-cli.adoc (1)

77-83: Missing step: set per-job allowOnline/allowCodeExecution on the LMEvalJob

Cluster-level switches alone are not sufficient. Users must also set the flags on their LMEvalJob resource.

 oc rollout restart deployment trustyai-service-operator-controller-manager -n redhat-ods-applications
 ----
+
+. Enable online access and/or code execution on the specific `LMEvalJob`:
++
+[source,sh]
+----
+# Patch an existing job (example name: example-lmeval)
+oc patch lmevaljob example-lmeval -n <user-or-target-namespace> \
+  --type merge -p '{"spec":{"allowOnline":true,"allowCodeExecution":true}}'
+----
++
+[source,yaml]
+----
+# Or define it in YAML when creating the job
+apiVersion: trustyai.opendatahub.io/v1alpha1
+kind: LMEvalJob
+metadata:
+  name: example-lmeval
+  namespace: <user-or-target-namespace>
+spec:
+  allowOnline: true
+  allowCodeExecution: true
+----
🧹 Nitpick comments (4)
assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1)

8-11: Tighten wording, link style, and clarify independent toggles

Minor grammar and clarity improvements; also make it clear that online access and code execution can be enabled independently.

- LMEval Jobs do not allow internet access or remote code execution by default. When configuring an `LMEvalJob`, it may require access to external resources, for example task datasets and model tokenizers, usually hosted on link:https://huggingface.co[Hugging Face]. If you trust the source and have reviewed the content of these artifacts, `LMEvalJob` can be configured to automatically download them.
+ LMEval jobs do not allow internet access or remote code execution by default. When configuring an `LMEvalJob`, it might require access to external resources, for example, task datasets and model tokenizers, usually hosted on link:https://huggingface.co[Hugging Face^]. If you trust the source and have reviewed the content of these artifacts, LMEval jobs can be configured to automatically download them.
 
-Follow the steps below to enable online access and remote code execution for LMEval Jobs. Choose to update these settings by using either the CLI or in the console.
+Follow the steps below to enable online access and/or remote code execution for LMEval jobs. Choose to update these settings by using either the CLI or the console. Enable only what you require for your use case.
modules/updating-lmeval-job-configuration-using-the-console.adoc (1)

53-66: Tighten step wording; remove duplication

The sentence duplicates “in the LMEvalJob custom resource”.

-. Add the following fields to the `LMEvalJob` custom resource to enable online access in the LMEvalJob custom resource:
+. Add the following fields to the `LMEvalJob` to enable online access and code execution for this job:
modules/updating-lmeval-job-configuration-using-the-cli.adoc (2)

9-9: Minor wording

“In the CLI” → “using the CLI”.

-Follow these steps to enable online access (`allowOnline`) and remote code execution (`allowCodeExecution`) modes in the CLI for LMEval Jobs. 
+Follow these steps to enable online access (`allowOnline`) and remote code execution (`allowCodeExecution`) modes using the CLI for LMEval Jobs. 

57-59: Clarify you can enable either capability independently

Readers may think both must be set to true together. Suggest clarifying “and/or” and showing how to set only one.

-. Update the settings for the TrustyAI operator to enable external connectivity and remote code execution:
+. Update the settings for the TrustyAI operator to enable external connectivity and/or remote code execution:
@@
-oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \
---type merge -p '{"data":{"lmes-allow-online":"true","lmes-allow-code-execution":"true"}}'
+oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \
+--type merge -p '{"data":{"lmes-allow-online":"true","lmes-allow-code-execution":"true"}}'
+# Example: enable online only
+# oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \
+# --type merge -p '{"data":{"lmes-allow-online":"true"}}'
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4284c5 and 2d75689.

⛔ Files ignored due to path filters (1)
  • .DS_Store is excluded by !**/.DS_Store
📒 Files selected for processing (5)
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1 hunks)
  • assemblies/evaluating-large-language-models.adoc (1 hunks)
  • modules/setting-up-lmeval.adoc (1 hunks)
  • modules/updating-lmeval-job-configuration-using-the-cli.adoc (1 hunks)
  • modules/updating-lmeval-job-configuration-using-the-console.adoc (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.
🔇 Additional comments (3)
assemblies/evaluating-large-language-models.adoc (2)

20-21: Placement of new section looks correct

Including the new section between setup and job execution reads well and keeps the flow logical.


20-21: Confirm assembly-in-assembly include is supported by your build pipeline

You’re including an ASSEMBLY file inside another ASSEMBLY. Some pipelines expect assemblies to include modules only. Please confirm this builds cleanly; otherwise, consider inlining the two procedure modules here or converting the included file to a CONCEPT/REFERENCE module.

modules/updating-lmeval-job-configuration-using-the-cli.adoc (1)

57-83: Add a post-patch verification snippet to confirm settings

To ensure your ConfigMap update and rollout took effect, append the following under the restart step in modules/updating-lmeval-job-configuration-using-the-cli.adoc:

[source,sh]

Verify ConfigMap values in your cluster

oc get cm trustyai-service-operator-config -n redhat-ods-applications
-o jsonpath='{.data.lmes-allow-online}{"\n"}{.data.lmes-allow-code-execution}{"\n"}'

Verify a sample LMEvalJob has the flags set

oc get lmevaljob -n
-o jsonpath='{.spec.allowOnline}{"\n"}{.spec.allowCodeExecution}{"\n"}'

Replace <example-lmeval-name> and <namespace> with your actual job name and namespace, then run these commands against your cluster to confirm both lmes-allow-online and lmes-allow-code-execution are enabled.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
modules/updating-lmeval-job-configuration-using-the-cli.adoc (4)

11-14: Enhance the security advisory with concrete risks and guidance

Consider briefly calling out typical risks (untrusted code execution, data exfiltration via outbound egress) and recommending mitigations (restrict egress via NetworkPolicy/EgressFirewall, use allowlists).

Apply this minimal tweak:

 [IMPORTANT]
 ====
-Enabling online access and code execution involves a security risk. Only use these configurations if you trust the source(s).
+Enabling online access and code execution introduces security risk (for example, untrusted code execution, outbound data exfiltration).
+Only enable these settings when strictly required, and prefer allowlisted sources with restricted egress (for example, NetworkPolicy/EgressFirewall).
 ====

39-53: Clarify the “Effect” of the opendatahub.io/managed annotation

“Allows the user to manage the TrustyAI operator” is vague. The practical effect is about reconcilers not overwriting manual edits to the resource.

 | `opendatahub.io/managed`
 | `true`
-| User changes are not allowed and will be automatically reverted. 
+| The ODH reconciler manages this resource; manual changes may be reverted automatically.
@@
 | `opendatahub.io/managed`
 | `false`
-| Allows the user to manage the TrustyAI operator.
+| Disables ODH reconciliation for this resource, allowing manual changes to persist.

62-76: Clarify that ConfigMap keys act as cluster-level gates; per-job flags are still required

Avoid the impression that setting the ConfigMap alone enables the features by default for all jobs.

 | `lmes-allow-online`
 | `false`
-| Enables internet access for LMEval jobs at the cluster level.
+| Permits (gates) internet access for LMEval jobs cluster-wide.
+| Effective only when `spec.allowOnline` is also set on the individual `LMEvalJob`.
@@
 | `lmes-allow-code-execution`
 | `false`
-| Allows execution of code downloaded alongside datasets or tokenizers.
+| Permits (gates) remote code execution for artifacts downloaded by LM-Eval (for example, datasets, tokenizers).
+| Effective only when `spec.allowCodeExecution` is also set on the `LMEvalJob`.

30-31: Avoid hard-coding the operator namespace in multiple commands

Consider defining a namespace attribute once (for example, :odh-app-namespace: redhat-ods-applications) and using -n {odh-app-namespace} across commands to reduce duplication and ease maintenance.

Also applies to: 58-59, 81-82

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cbb378 and 23ec8d0.

📒 Files selected for processing (4)
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1 hunks)
  • modules/setting-up-lmeval.adoc (2 hunks)
  • modules/updating-lmeval-job-configuration-using-the-cli.adoc (1 hunks)
  • modules/updating-lmeval-job-configuration-using-the-console.adoc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc
  • modules/setting-up-lmeval.adoc
  • modules/updating-lmeval-job-configuration-using-the-console.adoc
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.
🔇 Additional comments (3)
modules/updating-lmeval-job-configuration-using-the-cli.adoc (3)

26-37: ✅ Verified correct ConfigMap for LMEval settings

A repo-wide search confirms that:

  • The trustyai-service-operator-config ConfigMap is explicitly documented as the source for LM-Eval global settings (see modules/setting-up-lmeval.adoc:38).
  • Several LMEval modules reference and patch this ConfigMap (e.g. updating-lmeval-job-configuration-using-the-cli.adoc).
  • The opendatahub.io/managed annotation is used throughout to toggle operator management.

No changes required.


77-82: Ignore incorrect operator suggestion—TrustyAI operator manages LMEvalJob
LMEvalJob CRDs are reconciled by the TrustyAI Kubernetes operator (see modules/lmeval-evaluation-job.adoc), so restarting the trustyai-service-operator-controller-manager deployment is the correct step. No changes required.

Likely an incorrect or invalid review comment.


54-61: Double-check LMEval ConfigMap settings

  • The keys lmes-allow-online and lmes-allow-code-execution are used consistently across the documentation and match exactly.
  • These settings are only referenced in docs (e.g. modules/setting-up-lmeval.adoc, modules/updating-lmeval-job-configuration-using-the-cli.adoc).
  • There are no code references in this repo to validate the owning component; please manually confirm that the TrustyAI operator’s cluster-level ConfigMap named trustyai-service-operator-config actually defines these keys for controlling LMEval jobs.

@BSynRedhat BSynRedhat force-pushed the IENG-301_AllowOfflineText branch from c1ae17a to f9d521a Compare August 11, 2025 14:21
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
modules/setting-up-lmeval.adoc (1)

48-48: Typo (pre-existing): fix inline code formatting

Change “assign a value for --device argument” to “assign a value for the --device argument”.

🧹 Nitpick comments (4)
modules/setting-up-lmeval.adoc (2)

74-80: Defaults aligned and key name is correct; consider upstream/downstream gating and formatting booleans

  • Good: Using lmes-allow-code-execution matches the agreed key.
  • If upstream defaults differ, gate the table rows with ifdef/ifndef as needed; otherwise, leaving both as false is fine for downstream.
  • Minor: Keep boolean defaults consistently formatted as code literals.

Suggested optional gating and formatting adjustments:

+ifdef::upstream[]
 | `lmes-allow-online`
-| false
+| `true`
 | Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet. 
+endif::[]
+ifndef::upstream[]
 | `lmes-allow-online`
-| false
+| `false`
 | Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet. 
+endif::[]

- | `lmes-allow-code-execution`
- | false
+ifdef::upstream[]
+| `lmes-allow-code-execution`
+| `true`
+| Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+endif::[]
+ifndef::upstream[]
+| `lmes-allow-code-execution`
+| `false`
 | Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
+endif::[]

If upstream is not meant to be enabled by default, simply apply the code formatting part:

-| false
+| `false`
...
-| false
+| `false`

Note: Consistency with prior guidance that the correct key is lmes-allow-code-execution is maintained (per team learning). Would you like me to push the gated version if upstream truly differs?


86-89: Gate the IMPORTANT note to downstream if upstream defaults differ

The note is correct for product docs. If upstream defaults are not disabled, wrap this in ifndef::upstream[] to avoid confusing upstream readers.

+ifndef::upstream[]
 [IMPORTANT]
 --
 The `allowOnline` and `allowCodeExecution` settings are *disabled* by default at the operator level in {productname-long}. LMEval jobs do not have internet access or permission to run any externally downloaded code unless explicitly enabled.
 --
+endif::[]
modules/updating-lmeval-job-configuration-using-the-cli.adoc (2)

39-53: Remove stray tab in the table title

There’s a tab character between the filename and the descriptor; it may render oddly. Replace with a single space.

-.trustyai-service-operator-config	annotation keys for LMEval job access
+.trustyai-service-operator-config annotation keys for LMEval job access

89-92: Fix placeholder spacing to avoid copy/paste confusion

Remove the extra space before the closing > in the namespace placeholder.

-oc patch lmevaljobs example-lmeval -n <user-or-target-namespace > \
+oc patch lmevaljobs example-lmeval -n <user-or-target-namespace> \
 ...
-oc get lmevaljobs example-lmeval -n <user-or-target-namespace > \
+oc get lmevaljobs example-lmeval -n <user-or-target-namespace> \
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9d521a and 0e5b5a8.

📒 Files selected for processing (4)
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1 hunks)
  • modules/setting-up-lmeval.adoc (1 hunks)
  • modules/updating-lmeval-job-configuration-using-the-cli.adoc (1 hunks)
  • modules/updating-lmeval-job-configuration-using-the-console.adoc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • modules/updating-lmeval-job-configuration-using-the-console.adoc
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T15:08:58.799Z
Learnt from: RobGeada
PR: opendatahub-io/opendatahub-documentation#901
File: modules/setting-up-lmeval.adoc:102-104
Timestamp: 2025-08-08T15:08:58.799Z
Learning: In opendatahub-io/opendatahub-documentation, the correct ConfigMap key is `lmes-allow-code-execution` (not `lmes-code-execution`); ensure consistency across all modules and examples.

Applied to files:

  • modules/setting-up-lmeval.adoc
📚 Learning: 2025-07-16T09:05:50.422Z
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.

Applied to files:

  • modules/setting-up-lmeval.adoc
🔇 Additional comments (2)
modules/updating-lmeval-job-configuration-using-the-cli.adoc (2)

58-60: Good: correct ConfigMap keys and string-valued booleans

Patching lmes-allow-online and lmes-allow-code-execution with string booleans is correct for ConfigMap data.


69-76: Defaults table consistent with product defaults

Defaults shown as false for both keys align with the downstream/product stance.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
modules/setting-up-lmeval.adoc (1)

78-80: Update deprecated ConfigMap key and confirm defaults

  • Found one remaining use of the old key in
    modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:
    Line 68:
    "lmes-code-execution": "true",
    should be
    "lmes-allow-code-execution": "true",

  • All other references to lmes-allow-code-execution are correct:

    • modules/setting-up-lmeval.adoc (defaults to false)
    • modules/updating-lmeval-job-configuration-using-the-cli.adoc
    • modules/updating-lmeval-job-configuration-using-the-console.adoc

  • In setting-up-lmeval.adoc, the default of false is appropriate.
    If upstream examples should default to true, wrap the table rows in an ifdef::upstream[] block, e.g.:

    ifdef::upstream[]
    | `lmes-allow-code-execution`
    | `true`
    | Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
    endif::[]
    
    ifndef::upstream[]
    | `lmes-allow-code-execution`
    | `false`
    | Determines whether LMEval jobs can set the `trust remote code` mode to `on`.
    endif::[]

Please update the deprecated key and consider gating defaults if needed.

🧹 Nitpick comments (2)
modules/setting-up-lmeval.adoc (2)

48-48: Tighten code formatting for the CLI flag.

Use “the --device argument” (don’t include “argument” inside backticks).

-| Detect if there are GPUs available and assign a value for the `--device argument` for LM Evaluation Harness. If GPUs are available, the value is `cuda`. If there are no GPUs available, the value is `cpu`.
+| Detect if there are GPUs available and assign a value for the `--device` argument for LM Evaluation Harness. If GPUs are available, the value is `cuda`. If there are no GPUs available, the value is `cpu`.

74-77: Confirm intended defaults; consider upstream/downstream gating for allow-online.

Table sets lmes-allow-online default to false unconditionally. If upstream defaults should be enabled (as previously discussed), gate the rows by context; otherwise keep false for both.

-| `lmes-allow-online`
-| `false`
-| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet. 
+ifdef::upstream[]
+| `lmes-allow-online`
+| `true`
+| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet. 
+endif::[]
+ifndef::upstream[]
+| `lmes-allow-online`
+| `false`
+| Whether LMEval jobs can set the online mode to `on` to access artifacts (models, datasets, tokenizers) from the internet. 
+endif::[]
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5b5a8 and f95838e.

📒 Files selected for processing (4)
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1 hunks)
  • modules/setting-up-lmeval.adoc (2 hunks)
  • modules/updating-lmeval-job-configuration-using-the-cli.adoc (1 hunks)
  • modules/updating-lmeval-job-configuration-using-the-console.adoc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • modules/updating-lmeval-job-configuration-using-the-cli.adoc
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc
  • modules/updating-lmeval-job-configuration-using-the-console.adoc
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T15:08:58.799Z
Learnt from: RobGeada
PR: opendatahub-io/opendatahub-documentation#901
File: modules/setting-up-lmeval.adoc:102-104
Timestamp: 2025-08-08T15:08:58.799Z
Learning: In opendatahub-io/opendatahub-documentation, the correct ConfigMap key is `lmes-allow-code-execution` (not `lmes-code-execution`); ensure consistency across all modules and examples.

Applied to files:

  • modules/setting-up-lmeval.adoc
📚 Learning: 2025-07-16T09:05:50.422Z
Learnt from: bredamc
PR: opendatahub-io/opendatahub-documentation#860
File: modules/using-llm-as-a-judge-metrics-with-lmeval.adoc:62-65
Timestamp: 2025-07-16T09:05:50.422Z
Learning: In the opendatahub-documentation project, bredamc prefers comprehensive error detection in code reviews. Continue to point out potential errors whether they are related to the code changes in the PR or not, but clearly distinguish between errors introduced in the PR versus errors in existing code.

Applied to files:

  • modules/setting-up-lmeval.adoc

@BSynRedhat BSynRedhat force-pushed the IENG-301_AllowOfflineText branch from f95838e to 62a1f79 Compare August 11, 2025 15:44
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (10)
modules/updating-lmeval-job-configuration-using-the-web-console.adoc (3)

28-31: Standardize AsciiDoc source block syntax and add namespace to YAML example

Use [source,yaml] (no space) consistently, and include metadata.namespace in the example to avoid user confusion.

- [source, yaml]
+ [source,yaml]
----
 opendatahub.io/managed: "false"
----
- [source, yaml]
+ [source,yaml]
----
 lmes-allow-online: "true"
 lmes-allow-code-execution: "true"
----
- [source,yaml]
+ [source,yaml]
----
 apiVersion: trustyai.opendatahub.io/v1alpha1
 kind: LMEvalJob
 metadata:
   name: example-lmeval
+  namespace: <your-namespace>
 spec:
   allowOnline: true
   allowCodeExecution: true
----

Also applies to: 34-38, 55-66


51-54: Clarify which fields must be set per job

Spell out the field names so users know exactly what to set on the CR.

- Each LMEval job must explicitly be set to allow online access and code execution.
+ Each `LMEvalJob` resource must explicitly set `spec.allowOnline` and/or `spec.allowCodeExecution` to `true`.
- . Ensure that the following fields are set to `true` to enable online access and code execution for this job when writing your `LMEvalJob` custom resource:
+ . When writing your `LMEvalJob` custom resource, set `spec.allowOnline` and/or `spec.allowCodeExecution` to `true` for this job:

41-49: Add note: rollout restart doesn’t affect already running jobs

Users may expect live jobs to pick up changes; this clarifies behavior.

 . Click the *Actions* menu and select *Restart rollout*.
 +
 [NOTE]
 --
 Each LMEval job must explicitly be set to allow online access and code execution.
 --
++
+[NOTE]
+--
+The rollout restart updates the operator. Already running `LMEvalJob` pods will not be mutated; submit new jobs or re-run existing ones to pick up the new cluster-level settings.
+--
modules/updating-lmeval-job-configuration-using-the-cli.adoc (5)

6-6: Normalize title casing to match related module

Use “job” (lowercase) for consistency with the web console module title.

-= Updating LMEval Job configuration using the CLI
+= Updating LMEval job configuration using the CLI

21-22: Use the appropriate product reference for the OpenShift CLI

The link targets the OpenShift CLI (oc). Prefer explicit naming or {productname-long} to avoid ambiguity.

-* You have downloaded and installed the {productname-short} command-line interface (CLI). See link:https://docs.redhat.com/en/documentation/openshift_container_platform/{ocp-latest-version}/html/cli_tools/openshift-cli-oc#installing-openshift-cli[Installing the OpenShift CLI^].
+* You have downloaded and installed the OpenShift CLI (`oc`). See link:https://docs.redhat.com/en/documentation/openshift_container_platform/{ocp-latest-version}/html/cli_tools/openshift-cli-oc#installing-openshift-cli[Installing the OpenShift CLI^].

46-53: Clarify the effect of opendatahub.io/managed=false

Make the behavior explicit so users understand implications.

-| `opendatahub.io/managed`
-| `false`
-| Allows the user to manage the TrustyAI operator.
+| `opendatahub.io/managed`
+| `false`
+| Disables ODH auto-management for this ConfigMap so user changes persist. Use with caution.

58-60: Show independent toggling and disabling of settings

Demonstrate setting the keys independently and how to disable later.

 oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \
 --type merge -p '{"data":{"lmes-allow-online":"true","lmes-allow-code-execution":"true"}}'
----
+----
++
+[source,sh]
+----
+# Example: enable only online access
+oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \
+  --type merge -p '{"data":{"lmes-allow-online":"true"}}'
+
+# Example: disable code execution later
+oc patch configmap trustyai-service-operator-config -n redhat-ods-applications \
+  --type merge -p '{"data":{"lmes-allow-code-execution":"false"}}'
+----

77-82: Optional: validate operator rollout completion

Helps users ensure the change has been applied.

 oc rollout restart deployment trustyai-service-operator-controller-manager -n redhat-ods-applications
 ----
++
+[source,sh]
+----
+# Optional: wait for the rollout to complete
+oc rollout status deployment trustyai-service-operator-controller-manager -n redhat-ods-applications
+----
assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (2)

10-10: Tighten wording and name the “web console” explicitly

Small clarity improvement.

-Follow the steps below to enable online access and remote code execution for LMEval jobs. Choose to update these settings by using either the CLI or in the console. Enable one or both settings according to your needs.
+Follow the steps below to enable online access and/or remote code execution for LMEval jobs. Choose to update these settings using either the CLI or the web console.

7-11: Add a top-level security warning

Surface the risk at the assembly level so readers don’t miss it.

 [role='_abstract']
 LMEval jobs do not allow internet access or remote code execution by default. When configuring an `LMEvalJob`, it may require access to external resources, for example task datasets and model tokenizers, usually hosted on link:https://huggingface.co[Hugging Face]. If you trust the source and have reviewed the content of these artifacts, an `LMEvalJob` can be configured to automatically download them.
 
-Follow the steps below to enable online access and remote code execution for LMEval jobs. Choose to update these settings by using either the CLI or in the console. Enable one or both settings according to your needs.
+Follow the steps below to enable online access and/or remote code execution for LMEval jobs. Choose to update these settings using either the CLI or the web console.
+
+[IMPORTANT]
+====
+Enabling online access or remote code execution introduces security risk. Only enable these settings when you trust and have reviewed the external sources.
+====
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f95838e and 62a1f79.

⛔ Files ignored due to path filters (1)
  • .DS_Store is excluded by !**/.DS_Store
📒 Files selected for processing (5)
  • assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1 hunks)
  • assemblies/evaluating-large-language-models.adoc (1 hunks)
  • modules/setting-up-lmeval.adoc (2 hunks)
  • modules/updating-lmeval-job-configuration-using-the-cli.adoc (1 hunks)
  • modules/updating-lmeval-job-configuration-using-the-web-console.adoc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • assemblies/evaluating-large-language-models.adoc
  • modules/setting-up-lmeval.adoc
🔇 Additional comments (4)
modules/updating-lmeval-job-configuration-using-the-web-console.adoc (1)

75-82: Table descriptions look good; defaults align with procedural steps

No changes needed.

modules/updating-lmeval-job-configuration-using-the-cli.adoc (2)

89-93: Good: plural resource name and validation step

Correct use of lmevaljobs and the jsonpath verification is clear.


98-108: YAML example looks correct and aligned with other modules

apiVersion, kind, and spec flags are consistent. Including metadata.namespace is helpful.

assemblies/enabling-external-resource-access-for-lmeval-jobs.adoc (1)

12-15: Includes are correct and ordering is appropriate

No changes needed.

@obrown1205
Copy link
Contributor

LGTM!

@BSynRedhat BSynRedhat merged commit b86de8f into opendatahub-io:main Aug 11, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants